Fix assertion when using multiple data imports #2000
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are stored in vars, so we need to make sure that nlocals is large enough to account for that
|
I just ran into the same issue. It'd be really awesome if this were merged so I didn't have to resort to the hack of concatenating all my data files so I can import them all at once and access them as array members. Alternatively, if importing JSON data is supposed to be restricted to one file, that should be documented. |
|
I also ran into this just now, assume import "./a" as $a;
import "./b" as $b;
def a: $a;
def b: $b;When completed with either {a: a}or {b: b}it will evaluate. However, when completed with {a: a} * {b: b}it fails with That's quite confusing, and I spent quite a lot of time figuring out what's going on. Please fix this! |
|
LGTM, I will merge this patch with the following test, thank you. jq -n -c -L tests/modules 'import "data" as $a; import "data" as $b; def f: {$a, $b}; f' |
itchyny
added a commit
that referenced
this pull request
Jul 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fails an assertion:
Assertion failed: (var < fr->bc->nlocals), function frame_local_var, file src/execute.c, line 104.The issue appears to be that we don't properly increment the number of local vars defined in the frame where we
STORE_GLOBAL, which means we can easily have more vars stored than we expect in our assertions. The code forSTOREVdoes increment this, so I duplicated that logic intoSTORE_GLOBAL. I can't think of any ways that this might cause issues. @nicowilliams can you?